feat!: differentiate scalar parsing from byte arrays #194
+254
−288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AaronFeickert
force-pushed
the
scalar-byte-array
branch
from
7851f06 to
5d8b7f0
Compare
AaronFeickert
force-pushed
the
scalar-byte-array
branch
from
3f40bd1 to
7bcb57e
Compare
CjS77
reviewed
Aug 9, 2023
|
ACK |
AaronFeickert
force-pushed
the
scalar-byte-array
branch
2 times, most recently
from
ddc40c7 to
bec8195
Compare
There was a problem hiding this comment.
Nice, just some comments:
- Why force
from_bytesto use the underlyingfrom_canonical_bytescall? Why not add an additional methodfrom_canonical_bytesmethod to use the underlyingfrom_canonical_bytescall? This way the change will be non-breaking. - If the intention is to prohibit the underlying
from_bytes_mod_ordercall, I would propose changing the name offrom_bytestofrom_canonical_bytes.
|
@hansieodendaal thanks for the review.
There's no good reason to have scalar parsing use the underlying
Unfortunately implementing |
AaronFeickert
force-pushed
the
scalar-byte-array
branch
from
23196b3 to
26c3105
Compare
AaronFeickert
force-pushed
the
scalar-byte-array
branch
from
26c3105 to
7d10338
Compare
stringhandler
requested changes
Aug 26, 2023
|
After discussion, it sounds like the preferred approach is to rename the |
AaronFeickert
force-pushed
the
scalar-byte-array
branch
from
7d10338 to
77293d6
Compare
hansieodendaal
approved these changes
Sep 26, 2023
AaronFeickert
force-pushed
the
scalar-byte-array
branch
from
9269e2b to
e3279bf
Compare
AaronFeickert
force-pushed
the
scalar-byte-array
branch
from
e3279bf to
a000006
Compare
stringhandler
approved these changes
Sep 28, 2023
SWvheerden
added a commit
to tari-project/tari
that referenced
this pull request
Nov 2, 2023
Description --- Updates key parsing to differentiate between canonical- and reduction-based use cases. Makes several updates to account for `EpochTime` changes. Updates genesis data. Motivation and Context --- An [update](tari-project/tari-crypto#194) to `tari-crypto` differentiates secret key parsing between cases that should use canonical byte arrays, and those that use random byte arrays requiring modular wide reduction. This PR makes corresponding changes to account for this. It also makes several updates required since the `tari_utilities` release used here also makes breaking `EpochTime` changes that can't be cleanly done separately. How Has This Been Tested? --- Existing tests pass. Uses of parsed secret keys have been manually inspected for correctness. What process can a PR reviewer use to test or verify this change? --- Assert that secret keys and scalars instantiated from byte arrays use the appropriate approach: parsing from canonical byte arrays, or modular wide reduction from uniform byte arrays. Confirm that `EpochTime` changes reflect the intended logic, especially relating to timestamp and difficulty handling. BREAKING CHANGE: This modifies how secret keys and scalars are constructed and is therefore a breaking change. --------- Co-authored-by: Aaron Feickert <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, creating a scalar
RistrettoSecretKeyfrom a byte array performs modular reduction on 32 bytes. For cases where the input is intended to be canonical, this is suboptimal. For cases where the input is produced from a hashing operation, wide reduction should be used to mitigate bias.This work renames
SecretKey::from_bytestoSecretKey::from_canonical_bytesto support an underlyingByteArraytrait update. In the case ofRistrettoSecretKey, it uses the curve library's canonical parser and returns an error if the provided byte slice is not a canonical scalar encoding.It also adds a new
SecretKey::from_uniform_bytesfunction that uses wide reduction. For constructions like signatures and KDFs that use hashing operations to produce scalar values, this function is used and the underlying hashers are updated to produce 64-byte output in the case ofRistrettoSecretKey.It updates the Schnorr signature API to support raw signing and verification using challenge byte slices that are either canonical encodings or uniform. It renames several existing functions for clarity.
It corrects a few typos that were discovered along the way.
Closes #189.
BREAKING CHANGE: This changes the way that scalars are produced from byte arrays, modifies the
SecretKeytrait and correspondingRistrettoSecretKeyimplementation, and updates the Schnorr signature API.